Python: Preserve A2A response message ids#6053
Conversation
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
giles17
left a comment
giles17
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 93%
✓ Correctness
This PR correctly preserves A2A message IDs across three code paths: (1) terminal task history messages using an
orfallback from artifact_id to message_id, (2) in-progress status messages accessed viastatus.message.message_idinside an existing null guard, and (3) TaskStatusUpdateEvent messages viamessage.message_idalso within a null guard. TheAgentResponseUpdateclass already supportsmessage_id, and_process_updatepropagates it toMessageobjects inAgentResponse.from_updates. No correctness issues found.
✓ Security Reliability
This PR safely propagates A2A message IDs through the response pipeline. All changes are straightforward attribute passthrough with proper null safety via getattr defaults and pre-existing null guards. No security or reliability concerns identified.
✓ Test Coverage
The PR has solid test coverage for all three production code changes. Each code path that now preserves message_id (terminal task history messages, in-progress status messages, and TaskStatusUpdateEvent messages) has at least one corresponding test with a deterministic message_id and explicit assertion. The existing test for artifact-backed updates (test_streaming_terminal_task_artifacts_are_emitted_when_terminal_event_has_no_content, line 1403) validates the priority of artifact_id in the or-fallback expression. No meaningful coverage gaps identified.
✗ Design Approach
The task-based changes look consistent, but the broader design goal in the PR title/body is still incomplete: immediate
A2AMessageresponses continue to lose theirmessage_id, so only task/status responses are fixed by this approach.
Automated review by giles17's agents
| @@ -727,6 +729,7 @@ def _updates_from_task_update_event( | |||
| contents=contents, | |||
There was a problem hiding this comment.
This preserves IDs for TaskStatusUpdateEvent, but the immediate A2AMessage path still drops them. In _map_a2a_stream (lines 364-373), raw A2AMessage items are converted to AgentResponseUpdate with response_id but no message_id, so _process_update never copies the ID to the resulting Message. Direct message responses created via add_message_response(...) lose their message IDs in both streaming and non-streaming modes.
|
Closing in favor of #6163 |
Summary
message_idon streamed task status updatesmessage_idfor in-progress task messages and terminal task history messagesCloses #5949.
To verify
python -m py_compile python\packages\a2a\agent_framework_a2a\_agent.py python\packages\a2a\tests\test_a2a_agent.pyuv run pytest packages\a2a\tests\test_a2a_agent.py::test_streaming_single_working_update_with_message packages\a2a\tests\test_a2a_agent.py::test_streaming_status_update_event_yields_content packages\a2a\tests\test_a2a_agent.py::test_task_status_update_event_metadata_merged packages\a2a\tests\test_a2a_agent.py::test_history_message_metadata_propagated -quv run pytest packages\a2a\tests\test_a2a_agent.py -quv run ruff check packages\a2a\agent_framework_a2a\_agent.py packages\a2a\tests\test_a2a_agent.pygit diff --checkNote:
uv run ruff format --check packages\a2a\agent_framework_a2a\_agent.py packages\a2a\tests\test_a2a_agent.pyreports an existing formatting change in an unrelated test function signature. I left that untouched to avoid broad formatting churn.